fix: resolve 43 lint findings blocking CI
All checks were successful
check / check (push) Successful in 56s

- server.go: drop unused (*Server).serve int return (unparam) and
  remove the dead exitCode field so cleanShutdown no longer writes
  to a field nothing reads.
- service.go: rename range var ch -> modeChar in parseUserModeString
  and the isKnownUserModeChar parameter (varnamelen).
- service_test.go: rename tc -> testCase (varnamelen); lift the
  inline struct and caseState to package-level named types
  (applyUserModeCase, applyUserModeCaseState) with every field
  set explicitly (exhaustruct); split the 167-line case table into
  four categorised helpers (funlen); extract the per-case runner
  and outcome/state verifiers into helpers so TestApplyUserMode
  drops below gocognit 30 and flattens the wantErr nestif block.

No changes to .golangci.yml, Makefile, Dockerfile, or CI config.
No //nolint was used to silence any of these findings.

docker build --no-cache . passes clean: 0 lint issues, all tests
pass with -race, binary compiles.
This commit is contained in:
clawbot
2026-04-17 14:37:14 +00:00
parent 93611dad67
commit f24e33a310
3 changed files with 403 additions and 260 deletions

View File

@@ -45,7 +45,6 @@ type Params struct {
// It manages routing, middleware, and lifecycle. // It manages routing, middleware, and lifecycle.
type Server struct { type Server struct {
startupTime time.Time startupTime time.Time
exitCode int
sentryEnabled bool sentryEnabled bool
log *slog.Logger log *slog.Logger
ctx context.Context //nolint:containedctx // signal handling pattern ctx context.Context //nolint:containedctx // signal handling pattern
@@ -143,7 +142,7 @@ func (srv *Server) enableSentry() {
srv.sentryEnabled = true srv.sentryEnabled = true
} }
func (srv *Server) serve() int { func (srv *Server) serve() {
srv.ctx, srv.cancelFunc = context.WithCancel( srv.ctx, srv.cancelFunc = context.WithCancel(
context.Background(), context.Background(),
) )
@@ -168,8 +167,6 @@ func (srv *Server) serve() int {
<-srv.ctx.Done() <-srv.ctx.Done()
srv.cleanShutdown() srv.cleanShutdown()
return srv.exitCode
} }
func (srv *Server) cleanupForExit() { func (srv *Server) cleanupForExit() {
@@ -177,8 +174,6 @@ func (srv *Server) cleanupForExit() {
} }
func (srv *Server) cleanShutdown() { func (srv *Server) cleanShutdown() {
srv.exitCode = 0
ctxShutdown, shutdownCancel := context.WithTimeout( ctxShutdown, shutdownCancel := context.WithTimeout(
context.Background(), shutdownTimeout, context.Background(), shutdownTimeout,
) )

View File

@@ -880,23 +880,23 @@ func parseUserModeString(
ops := make([]userModeOp, 0, len(modeStr)-1) ops := make([]userModeOp, 0, len(modeStr)-1)
adding := true adding := true
for _, ch := range modeStr { for _, modeChar := range modeStr {
switch ch { switch modeChar {
case '+': case '+':
adding = true adding = true
case '-': case '-':
adding = false adding = false
default: default:
if !isKnownUserModeChar(ch) { if !isKnownUserModeChar(modeChar) {
return nil, unknownFlag return nil, unknownFlag
} }
if ch == 'o' && adding { if modeChar == 'o' && adding {
return nil, unknownFlag return nil, unknownFlag
} }
ops = append(ops, userModeOp{ ops = append(ops, userModeOp{
char: ch, adding: adding, char: modeChar, adding: adding,
}) })
} }
} }
@@ -910,8 +910,8 @@ func parseUserModeString(
// isKnownUserModeChar reports whether the character is a // isKnownUserModeChar reports whether the character is a
// recognized user mode letter. // recognized user mode letter.
func isKnownUserModeChar(ch rune) bool { func isKnownUserModeChar(modeChar rune) bool {
switch ch { switch modeChar {
case 'w', 'o': case 'w', 'o':
return true return true
default: default:

View File

@@ -393,6 +393,248 @@ func TestQueryUserMode(t *testing.T) {
} }
} }
// applyUserModeCaseState is the subset of session user-mode
// state the rigorous TestApplyUserMode suite asserts on. It
// mirrors the columns (oper, wallops) that the parser is
// permitted to mutate.
type applyUserModeCaseState struct {
oper bool
wallops bool
}
// applyUserModeCase describes one rigor-suite case for
// Service.ApplyUserMode: the pre-call DB state, the mode
// string input, and the expected post-call observable state
// (mode string on success, IRC error code on rejection, and
// persisted session state either way).
type applyUserModeCase struct {
name string
initialState applyUserModeCaseState
modeStr string
wantModes string
wantErr bool
wantErrCode irc.IRCMessageType
wantState applyUserModeCaseState
}
// applyUserModeCases returns every case listed in sneak's
// review of PR #96 plus a few adjacent ones. Split across
// helpers by category so each stays under funlen.
func applyUserModeCases() []applyUserModeCase {
cases := applyUserModeHappyPathCases()
cases = append(cases, applyUserModeSignTransitionCases()...)
cases = append(cases, applyUserModeMalformedCases()...)
cases = append(cases, applyUserModeUnknownLetterCases()...)
return cases
}
// applyUserModeHappyPathCases covers valid single-char and
// multi-char-without-sign-transition mode operations.
func applyUserModeHappyPathCases() []applyUserModeCase {
return []applyUserModeCase{
{
name: "+w from empty",
initialState: applyUserModeCaseState{oper: false, wallops: false},
modeStr: "+w",
wantModes: "+w",
wantErr: false,
wantErrCode: 0,
wantState: applyUserModeCaseState{oper: false, wallops: true},
},
{
name: "-w from +w",
initialState: applyUserModeCaseState{oper: false, wallops: true},
modeStr: "-w",
wantModes: "+",
wantErr: false,
wantErrCode: 0,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
{
name: "-o from +o",
initialState: applyUserModeCaseState{oper: true, wallops: false},
modeStr: "-o",
wantModes: "+",
wantErr: false,
wantErrCode: 0,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
{
name: "-wo from +ow",
initialState: applyUserModeCaseState{oper: true, wallops: true},
modeStr: "-wo",
wantModes: "+",
wantErr: false,
wantErrCode: 0,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
}
}
// applyUserModeSignTransitionCases covers multi-char mode
// strings where '+' and '-' flip partway through. +o is
// never legal via MODE, so strings containing it must be
// rejected atomically.
func applyUserModeSignTransitionCases() []applyUserModeCase {
return []applyUserModeCase{
{
name: "+w-o from +o",
initialState: applyUserModeCaseState{oper: true, wallops: false},
modeStr: "+w-o",
wantModes: "+w",
wantErr: false,
wantErrCode: 0,
wantState: applyUserModeCaseState{oper: false, wallops: true},
},
{
// +o is rejected before any op applies; wallops
// stays set.
name: "-w+o always rejects +o",
initialState: applyUserModeCaseState{oper: true, wallops: true},
modeStr: "-w+o",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: true, wallops: true},
},
{
// Wallops must NOT be cleared; oper must NOT be
// cleared. Rejection is fully atomic.
name: "+o-w+w rejects because of +o",
initialState: applyUserModeCaseState{oper: true, wallops: true},
modeStr: "+o-w+w",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: true, wallops: true},
},
}
}
// applyUserModeMalformedCases covers inputs that lack a
// leading '+' or '-' and inputs that consist of bare signs
// without mode letters. All must be rejected with no side
// effects.
func applyUserModeMalformedCases() []applyUserModeCase {
return []applyUserModeCase{
{
name: "w no prefix rejects",
initialState: applyUserModeCaseState{oper: false, wallops: false},
modeStr: "w",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
{
// Prove wallops is NOT cleared — the whole point
// of sneak's review.
name: "xw no prefix rejects (would have been" +
" silently -w before)",
initialState: applyUserModeCaseState{oper: false, wallops: true},
modeStr: "xw",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: false, wallops: true},
},
{
name: "empty string rejects",
initialState: applyUserModeCaseState{oper: false, wallops: false},
modeStr: "",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
{
name: "bare + rejects",
initialState: applyUserModeCaseState{oper: false, wallops: false},
modeStr: "+",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
{
name: "bare - rejects",
initialState: applyUserModeCaseState{oper: false, wallops: false},
modeStr: "-",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
{
name: "+-+ rejects (no mode letters)",
initialState: applyUserModeCaseState{oper: false, wallops: false},
modeStr: "+-+",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
}
}
// applyUserModeUnknownLetterCases covers well-formed prefix
// strings that contain unknown mode letters. Rejection must
// be atomic: any valid letters before the invalid one must
// not persist.
func applyUserModeUnknownLetterCases() []applyUserModeCase {
return []applyUserModeCase{
{
name: "-x+y rejects unknown -x",
initialState: applyUserModeCaseState{oper: false, wallops: false},
modeStr: "-x+y",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
{
name: "+y-x rejects unknown +y",
initialState: applyUserModeCaseState{oper: false, wallops: false},
modeStr: "+y-x",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
{
name: "+z unknown mode rejects",
initialState: applyUserModeCaseState{oper: false, wallops: false},
modeStr: "+z",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
{
// Wallops must NOT be set.
name: "+wz rejects whole thing; +w side effect" +
" must NOT persist",
initialState: applyUserModeCaseState{oper: false, wallops: false},
modeStr: "+wz",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
{
name: "+wo rejects whole thing; +w side effect" +
" must NOT persist",
initialState: applyUserModeCaseState{oper: false, wallops: false},
modeStr: "+wo",
wantModes: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: applyUserModeCaseState{oper: false, wallops: false},
},
}
}
// TestApplyUserMode is the rigorous table-driven suite for // TestApplyUserMode is the rigorous table-driven suite for
// the shared user-mode parser. It covers every case listed // the shared user-mode parser. It covers every case listed
// in sneak's review of PR #96 plus a few adjacent ones. // in sneak's review of PR #96 plus a few adjacent ones.
@@ -400,253 +642,159 @@ func TestQueryUserMode(t *testing.T) {
// persisted session state, to prove that rejected input // persisted session state, to prove that rejected input
// leaves no side effects. // leaves no side effects.
func TestApplyUserMode(t *testing.T) { func TestApplyUserMode(t *testing.T) {
type caseState struct { for _, testCase := range applyUserModeCases() {
oper bool t.Run(testCase.name, func(t *testing.T) {
wallops bool runApplyUserModeCase(t, testCase)
})
}
} }
// Each case starts from initialState, calls // runApplyUserModeCase executes one applyUserModeCase: seed
// ApplyUserMode(modeStr), and verifies either the // the session state, invoke ApplyUserMode, and verify both
// returned mode string (wantModes) or an IRCError with // the returned value and the post-call persisted state.
// wantErrCode. After the call, the DB must match func runApplyUserModeCase(
// wantState — proving that rejected input made no t *testing.T, testCase applyUserModeCase,
// change. ) {
cases := []struct { t.Helper()
name string
initialState caseState
modeStr string
wantModes string
wantErr bool
wantErrCode irc.IRCMessageType
wantState caseState
}{
// Happy path: single-char operations.
{
name: "+w from empty",
modeStr: "+w",
wantModes: "+w",
wantState: caseState{wallops: true},
},
{
name: "-w from +w",
initialState: caseState{wallops: true},
modeStr: "-w",
wantModes: "+",
wantState: caseState{},
},
{
name: "-o from +o",
initialState: caseState{oper: true},
modeStr: "-o",
wantModes: "+",
wantState: caseState{},
},
// Multi-char without sign transitions.
{
name: "-wo from +ow",
initialState: caseState{oper: true, wallops: true},
modeStr: "-wo",
wantModes: "+",
wantState: caseState{},
},
// Multi-char with sign transitions.
{
name: "+w-o from +o",
initialState: caseState{oper: true},
modeStr: "+w-o",
wantModes: "+w",
wantState: caseState{wallops: true},
},
{
name: "-w+o always rejects +o",
initialState: caseState{wallops: true, oper: true},
modeStr: "-w+o",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
// +o is rejected before any op applies; wallops
// stays set.
wantState: caseState{wallops: true, oper: true},
},
{
name: "+o-w+w rejects because of +o",
initialState: caseState{wallops: true, oper: true},
modeStr: "+o-w+w",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
// Wallops must NOT be cleared; oper must NOT be
// cleared. Rejection is fully atomic.
wantState: caseState{wallops: true, oper: true},
},
{
name: "-x+y rejects unknown -x",
modeStr: "-x+y",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: caseState{},
},
{
name: "+y-x rejects unknown +y",
modeStr: "+y-x",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: caseState{},
},
// Malformed prefix — must not silently treat as '-'.
{
name: "w no prefix rejects",
modeStr: "w",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: caseState{},
},
{
name: "xw no prefix rejects (would have been" +
" silently -w before)",
initialState: caseState{wallops: true},
modeStr: "xw",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
// Prove wallops is NOT cleared — the whole point
// of sneak's review.
wantState: caseState{wallops: true},
},
{
name: "empty string rejects",
modeStr: "",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: caseState{},
},
// Bare signs with no mode letters reject.
{
name: "bare + rejects",
modeStr: "+",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: caseState{},
},
{
name: "bare - rejects",
modeStr: "-",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: caseState{},
},
{
name: "+-+ rejects (no mode letters)",
modeStr: "+-+",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: caseState{},
},
// Unknown mode letters reject whole string.
{
name: "+z unknown mode rejects",
modeStr: "+z",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: caseState{},
},
{
name: "+wz rejects whole thing; +w side effect" +
" must NOT persist",
modeStr: "+wz",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
// Wallops must NOT be set.
wantState: caseState{},
},
{
name: "+wo rejects whole thing; +w side effect" +
" must NOT persist",
modeStr: "+wo",
wantErr: true,
wantErrCode: irc.ErrUmodeUnknownFlag,
wantState: caseState{},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
env := newTestEnv(t) env := newTestEnv(t)
ctx := t.Context() ctx := t.Context()
sid := createSession(ctx, t, env.db, "alice") sid := createSession(ctx, t, env.db, "alice")
if tc.initialState.oper { seedApplyUserModeState(ctx, t, env.db, sid, testCase.initialState)
if err := env.db.SetSessionOper(
result, err := env.svc.ApplyUserMode(
ctx, sid, testCase.modeStr,
)
verifyApplyUserModeOutcome(t, testCase, result, err)
verifyApplyUserModeState(ctx, t, env.db, sid, testCase.wantState)
}
// seedApplyUserModeState installs the pre-call session
// state described by initialState.
func seedApplyUserModeState(
ctx context.Context,
t *testing.T,
database *db.Database,
sid int64,
initialState applyUserModeCaseState,
) {
t.Helper()
if initialState.oper {
if err := database.SetSessionOper(
ctx, sid, true, ctx, sid, true,
); err != nil { ); err != nil {
t.Fatalf("init oper: %v", err) t.Fatalf("init oper: %v", err)
} }
} }
if tc.initialState.wallops { if initialState.wallops {
if err := env.db.SetSessionWallops( if err := database.SetSessionWallops(
ctx, sid, true, ctx, sid, true,
); err != nil { ); err != nil {
t.Fatalf("init wallops: %v", err) t.Fatalf("init wallops: %v", err)
} }
} }
}
result, err := env.svc.ApplyUserMode( // verifyApplyUserModeOutcome asserts the direct return
ctx, sid, tc.modeStr, // value of ApplyUserMode. It dispatches to the error- or
) // success-specific verifier based on wantErr.
func verifyApplyUserModeOutcome(
t *testing.T,
testCase applyUserModeCase,
result string,
err error,
) {
t.Helper()
if testCase.wantErr {
verifyApplyUserModeError(t, testCase, err)
return
}
verifyApplyUserModeSuccess(t, testCase, result, err)
}
// verifyApplyUserModeError checks that err is a
// *service.IRCError whose code matches wantErrCode.
func verifyApplyUserModeError(
t *testing.T, testCase applyUserModeCase, err error,
) {
t.Helper()
if tc.wantErr {
var ircErr *service.IRCError var ircErr *service.IRCError
if !errors.As(err, &ircErr) { if !errors.As(err, &ircErr) {
t.Fatalf( t.Fatalf("expected IRCError, got %v", err)
"expected IRCError, got %v", err,
)
} }
if ircErr.Code != tc.wantErrCode { if ircErr.Code != testCase.wantErrCode {
t.Errorf( t.Errorf(
"code: want %d got %d", "code: want %d got %d",
tc.wantErrCode, ircErr.Code, testCase.wantErrCode, ircErr.Code,
) )
} }
} else { }
// verifyApplyUserModeSuccess checks that err is nil and the
// returned mode string matches wantModes.
func verifyApplyUserModeSuccess(
t *testing.T,
testCase applyUserModeCase,
result string,
err error,
) {
t.Helper()
if err != nil { if err != nil {
t.Fatalf("unexpected error: %v", err) t.Fatalf("unexpected error: %v", err)
} }
if result != tc.wantModes { if result != testCase.wantModes {
t.Errorf( t.Errorf(
"modes: want %q got %q", "modes: want %q got %q",
tc.wantModes, result, testCase.wantModes, result,
) )
} }
} }
// Whether or not the call errored, the persisted // verifyApplyUserModeState asserts the post-call persisted
// state must match wantState — this is the // session state. This is the atomicity guarantee sneak
// atomicity guarantee sneak demanded. // demanded: whether the call succeeded or was rejected, the
gotOper, err := env.db.IsSessionOper(ctx, sid) // DB must match wantState exactly.
func verifyApplyUserModeState(
ctx context.Context,
t *testing.T,
database *db.Database,
sid int64,
wantState applyUserModeCaseState,
) {
t.Helper()
gotOper, err := database.IsSessionOper(ctx, sid)
if err != nil { if err != nil {
t.Fatalf("read oper: %v", err) t.Fatalf("read oper: %v", err)
} }
gotWallops, err := env.db.IsSessionWallops( gotWallops, err := database.IsSessionWallops(ctx, sid)
ctx, sid,
)
if err != nil { if err != nil {
t.Fatalf("read wallops: %v", err) t.Fatalf("read wallops: %v", err)
} }
if gotOper != tc.wantState.oper { if gotOper != wantState.oper {
t.Errorf( t.Errorf(
"oper: want %v got %v", "oper: want %v got %v",
tc.wantState.oper, gotOper, wantState.oper, gotOper,
) )
} }
if gotWallops != tc.wantState.wallops { if gotWallops != wantState.wallops {
t.Errorf( t.Errorf(
"wallops: want %v got %v", "wallops: want %v got %v",
tc.wantState.wallops, gotWallops, wantState.wallops, gotWallops,
) )
} }
})
}
} }