lint issues in codebase

[krtkvrm]

running golangci-lint fetched these lint errors

$ golangci-lint run

.
.
.

62 issues:
* errcheck: 50
* ineffassign: 2
* staticcheck: 7
* unused: 3

we should ideally solve this, not urgently but in trenches

raw log:

internal/jsonrpc2/conn.go:517:20: Error return value of `c.internalErrorf` is not checked (errcheck)
                        c.internalErrorf("Read returned an unexpected message of type %T", msg)
                                        ^
internal/jsonrpc2/conn.go:572:18: Error return value of `c.processResult` is not checked (errcheck)
                c.processResult("acceptRequest", req, nil, err)
                               ^
internal/jsonrpc2/conn.go:585:19: Error return value of `c.processResult` is not checked (errcheck)
                        c.processResult("Preempt", req, result, err)
                                       ^
internal/jsonrpc2/conn.go:625:18: Error return value of `c.processResult` is not checked (errcheck)
                c.processResult("acceptRequest", req, nil, err)
                               ^
internal/jsonrpc2/conn.go:677:19: Error return value of `c.internalErrorf` is not checked (errcheck)
                c.internalErrorf("%#v returned a non-nil result with a non-nil error for %s:\n%v\n%#v", from, req.Method, err, result)
                                ^
internal/jsonrpc2/jsonrpc2_test.go:139:17: Error return value of `listener.Close` is not checked (errcheck)
                listener.Close()
                              ^
internal/jsonrpc2/jsonrpc2_test.go:140:14: Error return value of `server.Wait` is not checked (errcheck)
                server.Wait()
                           ^
internal/jsonrpc2/jsonrpc2_test.go:149:24: Error return value of `h.conn.Close` is not checked (errcheck)
                                        defer h.conn.Close()
                                                          ^
internal/jsonrpc2/jsonrpc2_test.go:159:15: Error return value of `client.Wait` is not checked (errcheck)
                        client.Wait()
                                   ^
internal/jsonrpc2/jsonrpc2_test.go:378:19: Error return value of `h.conn.Respond` is not checked (errcheck)
                                h.conn.Respond(req.ID, true, nil)
                                              ^
internal/jsonrpc2/jsonrpc2_test.go:380:19: Error return value of `h.conn.Respond` is not checked (errcheck)
                                h.conn.Respond(req.ID, nil, ctx.Err())
                                              ^
internal/jsonrpc2/net.go:134:15: Error return value of `client.Close` is not checked (errcheck)
                client.Close()
                            ^
internal/jsonrpc2/net.go:135:15: Error return value of `server.Close` is not checked (errcheck)
                server.Close()
                            ^
internal/jsonrpc2/serve.go:94:19: Error return value of `s.listener.Close` is not checked (errcheck)
                s.listener.Close()
                                ^
internal/jsonrpc2/serve.go:183:13: Error return value of `rwc.Close` is not checked (errcheck)
                        rwc.Close()
                                 ^
internal/jsonrpc2/serve.go:208:13: Error return value of `rwc.Close` is not checked (errcheck)
                        rwc.Close()
                                 ^
internal/jsonrpc2/serve.go:270:17: Error return value of `l.wrapped.Close` is not checked (errcheck)
        l.wrapped.Close()
                       ^
internal/jsonrpc2/serve_test.go:52:23: Error return value of `listener.Close` is not checked (errcheck)
                defer listener.Close()
                                    ^
internal/jsonrpc2/serve_test.go:75:15: Error return value of `conn1.Close` is not checked (errcheck)
                        conn1.Close()
                                   ^
internal/jsonrpc2/serve_test.go:84:15: Error return value of `conn1.Close` is not checked (errcheck)
                        conn1.Close()
                                   ^
internal/jsonrpc2/serve_test.go:122:15: Error return value of `conn3.Close` is not checked (errcheck)
                        conn3.Close()
                                   ^
internal/jsonrpc2/serve_test.go:226:14: Error return value of `server.Wait` is not checked (errcheck)
                server.Wait()
                           ^
internal/jsonrpc2/serve_test.go:261:18: Error return value of `listener.Close` is not checked (errcheck)
                        listener.Close()
                                      ^
internal/jsonrpc2/serve_test.go:263:15: Error return value of `conn.Close` is not checked (errcheck)
                                conn.Close()
                                          ^
internal/jsonrpc2/serve_test.go:273:11: Error return value of `c.Close` is not checked (errcheck)
                        c.Close()
                               ^
internal/jsonrpc2/serve_test.go:326:10: Error return value of `s.Wait` is not checked (errcheck)
                        s.Wait()
                              ^
internal/jsonrpc2/serve_test.go:352:9: Error return value of `s.Wait` is not checked (errcheck)
                s.Wait()
                      ^
mcp/event_test.go:137:20: Error return value of `s.SessionClosed` is not checked (errcheck)
                                s.SessionClosed(ctx, "S1")
                                               ^
mcp/event_test.go:209:10: Error return value of `s.Append` is not checked (errcheck)
        s.Append(ctx, "S1", 1, []byte("d1"))
                ^
mcp/event_test.go:210:10: Error return value of `s.Append` is not checked (errcheck)
        s.Append(ctx, "S1", 1, []byte("d2"))
                ^
mcp/event_test.go:211:10: Error return value of `s.Append` is not checked (errcheck)
        s.Append(ctx, "S1", 1, []byte("d3"))
                ^
mcp/mcp_test.go:419:20: Error return value of `ss.NotifyProgress` is not checked (errcheck)
                ss.NotifyProgress(ctx, &ProgressNotificationParams{
                                 ^
mcp/mcp_test.go:426:20: Error return value of `cs.NotifyProgress` is not checked (errcheck)
                cs.NotifyProgress(ctx, &ProgressNotificationParams{
                                 ^
mcp/mcp_test.go:443:20: Error return value of `s.ResourceUpdated` is not checked (errcheck)
                s.ResourceUpdated(ctx, &ResourceUpdatedNotificationParams{
                                 ^
mcp/mcp_test.go:456:20: Error return value of `s.ResourceUpdated` is not checked (errcheck)
                s.ResourceUpdated(ctx, &ResourceUpdatedNotificationParams{
                                 ^
mcp/mcp_test.go:467:10: Error return value of `cs.Close` is not checked (errcheck)
        cs.Close()
                ^
mcp/mcp_test.go:569:16: Error return value of `cs.Close` is not checked (errcheck)
        defer cs.Close()
                      ^
mcp/mcp_test.go:586:10: Error return value of `cc.Close` is not checked (errcheck)
        cc.Close()
                ^
mcp/mcp_test.go:615:16: Error return value of `cs.Close` is not checked (errcheck)
        defer cs.Close()
                      ^
mcp/mcp_test.go:656:16: Error return value of `cs.CallTool` is not checked (errcheck)
        go cs.CallTool(ctx, &CallToolParams{Name: "slow"})
                      ^
mcp/mcp_test.go:807:9: Error return value of `ss.Wait` is not checked (errcheck)
        ss.Wait()
               ^
mcp/mcp_test.go:822:15: Error return value of `fmt.Fprintf` is not checked (errcheck)
                        fmt.Fprintf(w, "%s >%s\n", prefix, method)
                                   ^
mcp/mcp_test.go:823:21: Error return value of `fmt.Fprintf` is not checked (errcheck)
                        defer fmt.Fprintf(w, "%s <%s\n", prefix, method)
                                         ^
mcp/mcp_test.go:852:16: Error return value of `ss.Close` is not checked (errcheck)
        defer ss.Close()
                      ^
mcp/mcp_test.go:912:10: Error return value of `ss.Close` is not checked (errcheck)
        ss.Close()
                ^
mcp/resource_go124.go:21:15: Error return value of `r.Close` is not checked (errcheck)
        defer r.Close()
                     ^
mcp/server.go:533:11: Error return value of `ss.Close` is not checked (errcheck)
                ss.Close()
                        ^
mcp/sse.go:368:18: Error return value of `resp.Body.Close` is not checked (errcheck)
                resp.Body.Close()
                               ^
mcp/sse.go:383:16: Error return value of `s.Close` is not checked (errcheck)
                defer s.Close() // close the transport when the GET exits
                             ^
mcp/sse.go:464:23: Error return value of `resp.Body.Close` is not checked (errcheck)
        defer resp.Body.Close()
                             ^
internal/jsonrpc2/conn.go:697:5: ineffectual assignment to err (ineffassign)
                                err = writeErr
                                ^
internal/jsonrpc2/conn.go:700:4: ineffectual assignment to err (ineffassign)
                        err = c.internalErrorf("%#v returned a malformed result for %q: %w", from, req.Method, respErr)
                        ^
internal/jsonrpc2/conn.go:368:11: SA9003: empty branch (staticcheck)
                        } else {
                               ^
internal/jsonrpc2/conn.go:511:12: SA9003: empty branch (staticcheck)
                                } else {
                                       ^
internal/jsonrpc2/conn.go:695:4: SA4006: this value of writeErr is never used (staticcheck)
                        writeErr := c.write(notDone{req.ctx}, response)
                        ^
internal/jsonrpc2/conn.go:708:3: SA9003: empty branch (staticcheck)
                if err != nil {
                ^
mcp/streamable.go:115:3: S1005: unnecessary assignment to the blank identifier (staticcheck)
                session, _ = h.sessions[id]
                ^
mcp/streamable_test.go:425:29: SA4009: argument ctx is overwritten before first use (staticcheck)
                        tool: func(t *testing.T, ctx context.Context, ss *ServerSession) {
                                                 ^
mcp/streamable_test.go:428:5: SA4009(related information): assignment to ctx (staticcheck)
                                ctx = context.Background()
                                ^
mcp/mcp_test.go:491:2: var resource3 is unused (unused)
        resource3 = &Resource{
        ^
mcp/mcp_test.go:499:5: var embeddedResources is unused (unused)
var embeddedResources = map[string]string{
    ^
mcp/mcp_test.go:503:6: func handleEmbeddedResource is unused (unused)
func handleEmbeddedResource(_ context.Context, _ *ServerSession, params *ReadResourceParams) (*ReadResourceResult, error) {
     ^

[findleyr]

Thanks, I'll take a look through these.

(among other things, I think there is a fair bit of jsonrpc2 code that we can delete as it's not used in this package; I'd prefer to delete it rather than update it)

[krtkvrm]

along with this, I'll open a PR to add a Makefile to run these verifications locally and add some in github workflows @findleyr

[findleyr]

@krtkvrm I don't think the issues detected above are necessarily bugs, so I don't want to run this check as part of CI. However, it would be good to run a more conservative lint pass. Certainly we could run go vet, along perhaps with staticcheck?

Can we achieve this without a makefile?

[krtkvrm]

yes, we can definitely achieve this without a Makefile - just run the commands directly in CI and document them in the readme.
I personally think a simple Makefile would be nice for contributor experience, like make lint

[findleyr]

Reopening as the associated CL didn't address the issues, it simply added a vet step.

I think we should run staticcheck. I'll look into this now.