Skip to content

Commit eb64d07

Browse files
aryekocursoragent
andcommitted
fix: resolve all golangci-lint issues
- Security: Add ReadHeaderTimeout to HTTP server to prevent Slowloris attacks - Documentation: Add package comments and documentation for all exported types and functions - Performance: Pass large structs by pointer, optimize range loops to use indexing - Quality: Fix unused parameters in tests, use http.NoBody instead of nil - Code style: Combine duplicate string parameters in function signatures - Config: Add nolint directives for intentional design choices (API naming, test patterns, stdlib signatures) All tests pass and linter is clean. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent a3372c0 commit eb64d07

25 files changed

Lines changed: 132 additions & 66 deletions

.golangci.yml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,21 @@ issues:
6969
- funlen
7070
- gocyclo
7171
- lll
72+
73+
# Intentional API design choices
74+
- path: modkit/module/module\.go
75+
text: "type name will be used as module.ModuleDef"
76+
linters:
77+
- revive
78+
79+
# Test helpers that intentionally use value receivers
80+
- path: modkit/kernel/mod_helper_test\.go
81+
text: "hugeParam.*valueModule"
82+
linters:
83+
- gocritic
84+
85+
# Standard library types passed by value
86+
- path: modkit/logging/logger_test\.go
87+
text: "hugeParam.*slog.Record"
88+
linters:
89+
- gocritic

go.work.sum

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9 h1:VpgP7xuJadIUu
2222
github.com/AdamKorcz/go-118-fuzz-build v0.0.0-20230306123547-8075edf89bb0 h1:59MxjQVfjXsBpLy+dbd2/ELV5ofnUkUZBvWSC85sheA=
2323
github.com/AdamKorcz/go-118-fuzz-build v0.0.0-20230306123547-8075edf89bb0/go.mod h1:OahwfttHWG6eJ0clwcfBAHoDI6X/LV/15hx/wlMZSrU=
2424
github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802 h1:1BDTz0u9nC3//pOCMdNH+CiXJVYJh5UQNCOBG7jbELc=
25+
github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww=
2526
github.com/Microsoft/hcsshim v0.11.5 h1:haEcLNpj9Ka1gd3B3tAEs9CpE0c+1IhoL59w/exYU38=
2627
github.com/Microsoft/hcsshim v0.11.5/go.mod h1:MV8xMfmECjl5HdO7U/3/hFVnkmSBjAjmA09d4bExKcU=
2728
github.com/PuerkitoBio/purell v1.1.1 h1:WEQqlqaGbrPkxLJWfBwQmfEAE1Z7ONdDLqrN38tNFfI=
@@ -351,11 +352,8 @@ golang.org/x/oauth2 v0.26.0 h1:afQXWNNaeC4nvZ0Ed9XvCCzXM6UHJG7iCg0W4fPqSBE=
351352
golang.org/x/oauth2 v0.26.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI=
352353
golang.org/x/sync v0.3.0 h1:ftCYgMx6zT/asHUrPw8BLLscYtGznsLAnjq5RH9P66E=
353354
golang.org/x/sync v0.17.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
354-
golang.org/x/text v0.33.0/go.mod h1:LuMebE6+rBincTi9+xWTY8TztLzKHc/9C1uBCG27+q8=
355355
golang.org/x/time v0.10.0 h1:3usCWA8tQn0L8+hFJQNgzpWbd89begxN66o1Ojdn5L4=
356356
golang.org/x/time v0.10.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
357-
golang.org/x/time v0.11.0 h1:/bpjEDfN9tkoN/ryeYHnv5hcMlc8ncjMcM4XBk5NWV0=
358-
golang.org/x/time v0.11.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg=
359357
golang.org/x/tools v0.37.0/go.mod h1:MBN5QPQtLMHVdvsbtarmTNukZDdgwdwlO5qGacAzF0w=
360358
golang.org/x/tools v0.40.0/go.mod h1:Ik/tzLRlbscWpqqMRjyWYDisX8bG13FrdXp3o4Sr9lc=
361359
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=

modkit/http/logging.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@ import (
44
"net/http"
55
"time"
66

7-
"github.com/go-chi/chi/v5/middleware"
7+
"github.com/go-chi/chi/v5/middleware" //nolint:goimports // False positive
88
"github.com/go-modkit/modkit/modkit/logging"
99
)
1010

11+
// RequestLogger returns an HTTP middleware that logs each request with method, path, status, and duration.
1112
func RequestLogger(logger logging.Logger) func(http.Handler) http.Handler {
1213
if logger == nil {
1314
logger = logging.NewNopLogger()

modkit/http/logging_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func TestRequestLogger_LogsRequests(t *testing.T) {
2525
})
2626

2727
rec := httptest.NewRecorder()
28-
req := httptest.NewRequest(http.MethodGet, "/health", nil)
28+
req := httptest.NewRequest(http.MethodGet, "/health", http.NoBody)
2929
router.ServeHTTP(rec, req)
3030

3131
if len(logger.messages) == 0 {

modkit/http/register_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ type testControllerB struct{ called bool }
1212

1313
func (c *testController) RegisterRoutes(router Router) {
1414
c.called = true
15-
router.Handle(http.MethodGet, "/ping", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
15+
router.Handle(http.MethodGet, "/ping", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
1616
w.WriteHeader(http.StatusNoContent)
1717
}))
1818
}
1919

20-
func (c *testControllerB) RegisterRoutes(router Router) {
20+
func (c *testControllerB) RegisterRoutes(_ Router) {
2121
c.called = true
2222
}
2323

@@ -26,7 +26,7 @@ type orderedController struct {
2626
order *[]string
2727
}
2828

29-
func (c *orderedController) RegisterRoutes(router Router) {
29+
func (c *orderedController) RegisterRoutes(_ Router) {
3030
*c.order = append(*c.order, c.name)
3131
}
3232

modkit/http/router.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ type routerAdapter struct {
2424
chi.Router
2525
}
2626

27-
func (r routerAdapter) Handle(method string, pattern string, handler http.Handler) {
27+
func (r routerAdapter) Handle(method, pattern string, handler http.Handler) {
2828
r.Method(method, pattern, handler)
2929
}
3030

modkit/http/router_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ import (
1010

1111
func TestNewRouter_AllowsRoute(t *testing.T) {
1212
router := NewRouter()
13-
router.Method(http.MethodGet, "/ping", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
13+
router.Method(http.MethodGet, "/ping", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
1414
w.WriteHeader(http.StatusNoContent)
1515
}))
1616

17-
req := httptest.NewRequest(http.MethodGet, "/ping", nil)
17+
req := httptest.NewRequest(http.MethodGet, "/ping", http.NoBody)
1818
rec := httptest.NewRecorder()
1919
router.ServeHTTP(rec, req)
2020

@@ -29,12 +29,12 @@ func TestRouterGroup_RegistersGroupedRoutes(t *testing.T) {
2929

3030
called := false
3131
r.Group("/api", func(sub Router) {
32-
sub.Handle(http.MethodGet, "/users", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
32+
sub.Handle(http.MethodGet, "/users", http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
3333
called = true
3434
}))
3535
})
3636

37-
req := httptest.NewRequest(http.MethodGet, "/api/users", nil)
37+
req := httptest.NewRequest(http.MethodGet, "/api/users", http.NoBody)
3838
rec := httptest.NewRecorder()
3939
router.ServeHTTP(rec, req)
4040

@@ -55,9 +55,9 @@ func TestRouterUse_AttachesMiddleware(t *testing.T) {
5555
})
5656
})
5757

58-
r.Handle(http.MethodGet, "/test", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
58+
r.Handle(http.MethodGet, "/test", http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {}))
5959

60-
req := httptest.NewRequest(http.MethodGet, "/test", nil)
60+
req := httptest.NewRequest(http.MethodGet, "/test", http.NoBody)
6161
rec := httptest.NewRecorder()
6262
router.ServeHTTP(rec, req)
6363

@@ -73,7 +73,7 @@ func TestRouterGroup_MiddlewareScopedToGroup(t *testing.T) {
7373
groupMiddlewareCalled := false
7474
groupHandlerCalled := false
7575

76-
r.Handle(http.MethodGet, "/public", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
76+
r.Handle(http.MethodGet, "/public", http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {}))
7777

7878
r.Group("/protected", func(sub Router) {
7979
sub.Use(func(next http.Handler) http.Handler {
@@ -82,19 +82,19 @@ func TestRouterGroup_MiddlewareScopedToGroup(t *testing.T) {
8282
next.ServeHTTP(w, req)
8383
})
8484
})
85-
sub.Handle(http.MethodGet, "/resource", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
85+
sub.Handle(http.MethodGet, "/resource", http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
8686
groupHandlerCalled = true
8787
}))
8888
})
8989

90-
reqPublic := httptest.NewRequest(http.MethodGet, "/public", nil)
90+
reqPublic := httptest.NewRequest(http.MethodGet, "/public", http.NoBody)
9191
router.ServeHTTP(httptest.NewRecorder(), reqPublic)
9292

9393
if groupMiddlewareCalled {
9494
t.Fatal("group middleware should not affect routes outside group")
9595
}
9696

97-
reqProtected := httptest.NewRequest(http.MethodGet, "/protected/resource", nil)
97+
reqProtected := httptest.NewRequest(http.MethodGet, "/protected/resource", http.NoBody)
9898
router.ServeHTTP(httptest.NewRecorder(), reqProtected)
9999

100100
if !groupMiddlewareCalled || !groupHandlerCalled {

modkit/http/server.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ var shutdownServer = func(ctx context.Context, server *http.Server) error {
2525
// It handles SIGINT and SIGTERM for graceful shutdown.
2626
func Serve(addr string, handler http.Handler) error {
2727
server := &http.Server{
28-
Addr: addr,
29-
Handler: handler,
28+
Addr: addr,
29+
Handler: handler,
30+
ReadHeaderTimeout: 15 * time.Second,
3031
}
3132

3233
sigCh := make(chan os.Signal, 1)

modkit/http/server_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestServe_ReturnsErrorWhenServerFailsToStart(t *testing.T) {
2828
gotHandler = server.Handler
2929
return errors.New("boom")
3030
}
31-
shutdownServer = func(ctx context.Context, server *http.Server) error {
31+
shutdownServer = func(_ context.Context, _ *http.Server) error {
3232
return nil
3333
}
3434

@@ -65,12 +65,12 @@ func TestServe_HandlesSignals_ReturnsNil(t *testing.T) {
6565
serveStarted := make(chan struct{})
6666
shutdownRequested := make(chan struct{})
6767

68-
listenAndServe = func(server *http.Server) error {
68+
listenAndServe = func(_ *http.Server) error {
6969
close(serveStarted)
7070
<-shutdownRequested
7171
return http.ErrServerClosed
7272
}
73-
shutdownServer = func(ctx context.Context, server *http.Server) error {
73+
shutdownServer = func(_ context.Context, _ *http.Server) error {
7474
close(shutdownRequested)
7575
return nil
7676
}
@@ -120,7 +120,7 @@ func TestServe_ShutdownWaitsForInFlightRequest(t *testing.T) {
120120
requestStarted := make(chan struct{})
121121
releaseRequest := make(chan struct{})
122122

123-
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
123+
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
124124
close(requestStarted)
125125
<-releaseRequest
126126
w.WriteHeader(http.StatusOK)

modkit/kernel/bootstrap.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Package kernel provides the dependency injection container and module bootstrapping logic.
12
package kernel
23

34
import (
@@ -7,6 +8,8 @@ import (
78
"github.com/go-modkit/modkit/modkit/module"
89
)
910

11+
// App represents a bootstrapped modkit application with its dependency graph,
12+
// container, and instantiated controllers.
1013
type App struct {
1114
Graph *Graph
1215
container *Container
@@ -17,6 +20,9 @@ func controllerKey(moduleName, controllerName string) string {
1720
return moduleName + ":" + controllerName
1821
}
1922

23+
// Bootstrap constructs a modkit application from a root module.
24+
// It builds the module graph, validates dependencies, creates the DI container,
25+
// and instantiates all controllers.
2026
func Bootstrap(root module.Module) (*App, error) {
2127
graph, err := BuildGraph(root)
2228
if err != nil {
@@ -35,7 +41,8 @@ func Bootstrap(root module.Module) (*App, error) {
3541

3642
controllers := make(map[string]any)
3743
perModule := make(map[string]map[string]bool)
38-
for _, node := range graph.Modules {
44+
for i := range graph.Modules {
45+
node := &graph.Modules[i]
3946
if perModule[node.Name] == nil {
4047
perModule[node.Name] = make(map[string]bool)
4148
}

0 commit comments

Comments
 (0)