chore(lint): no single-character names, enforced and swept repo-wide

AGENTS.md

@@ -240,5 +240,5 @@ Exit codes:
 - Errors are values: wrap with context, compare with `errors.Is`/`errors.As`, and never log and return the same error.
 - Comments say how a thing is used and move with the code. Document functions, not every line.
 - No magic numbers: a literal that carries meaning is a named const with a comment (`const maxRequestBody = 1 << 20 // 1 MiB cap on JSON bodies`); 0 and 1 as plain counters and indexes are fine. `mnd` in golangci-lint enforces it outside tests, and `make lint-layering` proves it bites.
-- Names are words where they have to carry meaning: in a long or complex function, name variables and parameters for what they hold (`request`, `writer`, `service`, `caller`), not one letter. Short, conventional names (`i` in a three-line loop, `ctx`, `err`) are fine where the whole scope fits on screen. This is review advice, not a lint rule.
+- No single-character names, anywhere: receivers, parameters, results, locals, loop and closure variables, type parameters, fields, in tests too. Name a thing for what it holds (`func (service *Service)`, `writer http.ResponseWriter`, `request *http.Request`, `test *testing.T`, `for index := range`); two letters are fine where they are the word (`ctx`, `id`, `ok`, `tx`). It reads better and makes code easy to search, for people and agents. `scripts/namelint` enforces it in `make lint`, `make check-fast` and CI (golangci-lint has no linter that catches every case), skipping generated code but not `.templ` sources, and `make lint-layering` proves it bites.
 - If a rule here fights the task in front of you, say so loudly and get a human sign-off before breaking it.

Makefile

@@ -64,9 +64,10 @@ check-generated: generate ## Fail if generated code is out of date
 	@test -z "$$(git status --porcelain --untracked-files=all -- '*.go')" || (git status --short -- '*.go'; echo 'untracked generated files: run make generate and commit'; exit 1)
 
 .PHONY: lint
-lint: ## gofmt + golangci-lint (incl. layering rules) + test file locations
+lint: ## gofmt + golangci-lint (incl. layering rules) + single-character names + test file locations
 	@test -z "$$(gofmt -l .)" || (gofmt -l . && echo 'gofmt: run gofmt -w .' && exit 1)
 	$(TOOL) golangci-lint run
+	$(GO) run ./scripts/namelint ./...
 	./scripts/check-test-boundaries.sh
 	./scripts/check-ui-api-paths.sh
 
@@ -75,7 +76,7 @@ check-migrations: ## Fail if a new migration is numbered at or below the highest
 	./scripts/check-migration-order.sh
 
 .PHONY: lint-layering
-lint-layering: ## Prove forbidden imports (code and tests) and magic numbers fail lint
+lint-layering: ## Prove forbidden imports (code and tests), magic numbers and single-character names fail lint
 	./scripts/check-layering.sh
 
 .PHONY: lint-tests
@@ -307,7 +308,8 @@ DEPLOY_KEEP ?=
 
 .PHONY: deploy-check
 deploy-check: ## vet and lint the deploy/ module (it has no tests), then deploy-vars
-	cd deploy && $(GO) vet ./... && (cd .railway && $(GO) vet railway.go) && $(GO) tool -modfile=../tools/go.mod golangci-lint run -c ../.golangci.yml ./...
+	$(GO) build -o bin/namelint ./scripts/namelint
+	cd deploy && $(GO) vet ./... && (cd .railway && $(GO) vet railway.go) && $(GO) tool -modfile=../tools/go.mod golangci-lint run -c ../.golangci.yml ./... && ../bin/namelint ./...
 	$(MAKE) --no-print-directory deploy-vars
 
 .PHONY: deploy-vars

apps/admin/cmd/admin/main.go

@@ -178,15 +178,15 @@ func invite(ctx context.Context, cfg config.Config, log *slog.Logger, email stri
 		return err
 	}
 	svc := services.NewAdminService(server.NewAdminAccounts(admins, adminrepo.NewAdmins(adminPool)), adminrepo.NewAuditLog(adminPool), policy(cfg))
-	a, err := svc.InviteAdmin(ctx, services.SystemActor("cli"), in)
+	admin, err := svc.InviteAdmin(ctx, services.SystemActor("cli"), in)
 	if err != nil {
 		return err
 	}
 	// Let the queued email go out before exiting.
 	if err := admins.Shutdown(ctx); err != nil {
 		return err
 	}
-	log.Info("invite sent", "email", a.Email, "set_password_page", config.Absolute(cfg.PublicURL, config.AppRoutes.SetPassword))
+	log.Info("invite sent", "email", admin.Email, "set_password_page", config.Absolute(cfg.PublicURL, config.AppRoutes.SetPassword))
 	return nil
 }
 
@@ -316,27 +316,27 @@ const rotationTimeout = 30 * time.Second
 // the URL rotates; a new URL must pass the migrations probe or the current
 // pool stays.
 func livePool(ctx context.Context, log *slog.Logger, name string, url func() (secrets.Secret, error), onChange func(secrets.Hook), migrated map[string]fs.FS) (*database.Pool, error) {
-	u, err := url()
+	secret, err := url()
 	if err != nil {
 		return nil, err
 	}
-	pool, err := database.OpenPool(ctx, reveal.Value(u), database.PoolOptions{Probe: database.MigratedProbe(migrated), Logger: log})
+	pool, err := database.OpenPool(ctx, reveal.Value(secret), database.PoolOptions{Probe: database.MigratedProbe(migrated), Logger: log})
 	if err != nil {
 		return nil, err
 	}
-	applied := u.Fingerprint()
+	applied := secret.Fingerprint()
 	log.Info("database: pool opened", "database", name, "url", applied)
-	onChange(func(v secrets.Secret) error {
-		if v.Fingerprint() == applied {
+	onChange(func(updated secrets.Secret) error {
+		if updated.Fingerprint() == applied {
 			return nil
 		}
 		ctx, cancel := context.WithTimeout(ctx, rotationTimeout) // shutdown aborts a rotation in progress
 		defer cancel()
-		if err := pool.Swap(ctx, reveal.Value(v)); err != nil {
+		if err := pool.Swap(ctx, reveal.Value(updated)); err != nil {
 			return err // the old pool stays; retried after the next fetch
 		}
-		log.Info("database: URL rotated", "database", name, "from", applied, "to", v.Fingerprint())
-		applied = v.Fingerprint()
+		log.Info("database: URL rotated", "database", name, "from", applied, "to", updated.Fingerprint())
+		applied = updated.Fingerprint()
 		return nil
 	})
 	return pool, nil
@@ -349,15 +349,15 @@ const reconcileInterval = 5 * time.Minute
 // reconcileAudit resolves pending audit entries (admin changes whose outcome
 // was never written) from the current state, at startup and then every
 // reconcileInterval, until ctx ends.
-func reconcileAudit(ctx context.Context, r *services.AuditReconciler, after time.Duration, log *slog.Logger) {
+func reconcileAudit(ctx context.Context, reconciler *services.AuditReconciler, after time.Duration, log *slog.Logger) {
 	tick := time.NewTicker(reconcileInterval)
 	defer tick.Stop()
 	for {
-		res, err := r.Reconcile(ctx, after)
+		res, err := reconciler.Reconcile(ctx, after)
 		if err != nil && ctx.Err() == nil {
 			log.Error("admin: audit reconcile incomplete; entries stay pending", "err", err, "skipped", res.Skipped)
 		}
-		if n := res.Succeeded + res.Failed + res.Unknown; n > 0 {
+		if count := res.Succeeded + res.Failed + res.Unknown; count > 0 {
 			log.Warn("admin: resolved audit entries whose outcome was never written",
 				"succeeded", res.Succeeded, "failed", res.Failed, "unknown", res.Unknown)
 		}
@@ -415,18 +415,18 @@ func newAdminAuth(ctx context.Context, cfg config.Config, adminPool auth.DB, eve
 		return nil, err
 	}
 	applied := keys.Fingerprint()
-	cfg.Secrets.OnAuthSecrets(func(k secrets.Keyring) error {
-		if k.Fingerprint() == applied {
+	cfg.Secrets.OnAuthSecrets(func(keyring secrets.Keyring) error {
+		if keyring.Fingerprint() == applied {
 			return nil
 		}
 		ctx, cancel := context.WithTimeout(ctx, rotationTimeout)
 		defer cancel()
-		current, previous, revoked := reveal.Keyring(k)
+		current, previous, revoked := reveal.Keyring(keyring)
 		if err := admins.SetSecrets(ctx, auth.Secrets{Current: current, Previous: previous, Revoked: revoked}); err != nil {
 			return err // the old keyring stays; retried after the next fetch
 		}
-		log.Info("auth: ADMIN_AUTH_SECRET rotated", "from", applied, "to", k.Fingerprint())
-		applied = k.Fingerprint()
+		log.Info("auth: ADMIN_AUTH_SECRET rotated", "from", applied, "to", keyring.Fingerprint())
+		applied = keyring.Fingerprint()
 		return nil
 	})
 	return admins, nil

apps/admin/internal/api/admins_test.go

@@ -22,82 +22,82 @@ var (
 
 var adminRoutes = map[string]route{
 	"GET admins": {
-		method: "GET", path: r.Admins,
-		expect: func(m svcs, err error) {
-			m.admins.ListAdmins(anyCtx, actor).Return([]services.Admin{{ID: "a1", Email: grace.Email, HasPassword: true, TwoFactorEnabled: true}, linus}, err)
+		method: "GET", path: routes.Admins,
+		expect: func(svcs svcs, err error) {
+			svcs.admins.ListAdmins(anyCtx, actor).Return([]services.Admin{{ID: "a1", Email: grace.Email, HasPassword: true, TwoFactorEnabled: true}, linus}, err)
 		},
-		check: func(t *testing.T, res apitest.Response) {
-			got := apitest.Data[[]services.Admin](t, res)
+		check: func(test *testing.T, res apitest.Response) {
+			got := apitest.Data[[]services.Admin](test, res)
 			if len(got) != 2 || !got[0].HasPassword || got[1].HasPassword || got[1].Email != linus.Email {
-				t.Errorf("data = %s", res.Data)
+				test.Errorf("data = %s", res.Data)
 			}
 		},
 	},
 	"POST admin invite": {
-		method: "POST", path: r.AdminInvites, body: `{"email":"  Linus@Example.COM ","name":"  Linus T "}`,
+		method: "POST", path: routes.AdminInvites, body: `{"email":"  Linus@Example.COM ","name":"  Linus T "}`,
 		invalidBody: `{"email":"not an email"}`, field: "email",
 		errs: []svcErr{{services.ErrAdminExists, 409, "This person is already an admin"}, unavailable},
-		expect: func(m svcs, err error) {
-			m.admins.InviteAdmin(anyCtx, actor, validate.CreateAdminInvite{Email: "linus@example.com", Name: "Linus T"}).Return(linus, err)
+		expect: func(svcs svcs, err error) {
+			svcs.admins.InviteAdmin(anyCtx, actor, validate.CreateAdminInvite{Email: "linus@example.com", Name: "Linus T"}).Return(linus, err)
 		},
-		check: func(t *testing.T, res apitest.Response) {
-			if got := apitest.Data[services.Admin](t, res); got != linus {
-				t.Errorf("data = %s", res.Data)
+		check: func(test *testing.T, res apitest.Response) {
+			if got := apitest.Data[services.Admin](test, res); got != linus {
+				test.Errorf("data = %s", res.Data)
 			}
 		},
 	},
 	"POST admin disable": {
 		method: "POST", path: config.AdminPath("admin 2", "/disable"),
 		errs: []svcErr{adminNotFound, adminSelf, lastAdmin, {services.ErrAdminDisabled, 400, "This admin is already disabled"}, unavailable},
-		expect: func(m svcs, err error) { // the path id arrives unescaped
-			m.admins.DisableAdmin(anyCtx, actor, "admin 2").Return(err)
+		expect: func(svcs svcs, err error) { // the path id arrives unescaped
+			svcs.admins.DisableAdmin(anyCtx, actor, "admin 2").Return(err)
 		},
 		check: wantSuccess,
 	},
 	"POST admin enable": {
 		method: "POST", path: config.AdminPath("a2", "/enable"),
 		errs: []svcErr{adminNotFound, {services.ErrAdminNotDisabled, 400, "This admin is not disabled"}, unavailable},
-		expect: func(m svcs, err error) {
-			m.admins.EnableAdmin(anyCtx, actor, "a2").Return(err)
+		expect: func(svcs svcs, err error) {
+			svcs.admins.EnableAdmin(anyCtx, actor, "a2").Return(err)
 		},
 		check: wantSuccess,
 	},
 	"POST admin password reset": {
 		method: "POST", path: config.AdminPath("a2", "/password-reset"),
 		errs: []svcErr{adminNotFound, {services.ErrAdminPending, 400, services.ErrAdminPending.Message}, {services.ErrAdminSelfReset, 400, services.ErrAdminSelfReset.Message}, unavailable},
-		expect: func(m svcs, err error) {
-			m.admins.SendPasswordReset(anyCtx, actor, "a2").Return(err)
+		expect: func(svcs svcs, err error) {
+			svcs.admins.SendPasswordReset(anyCtx, actor, "a2").Return(err)
 		},
 		check: wantSuccess,
 	},
 	"POST admin two-factor reset": {
 		method: "POST", path: config.AdminPath("a2", "/two-factor/reset"),
 		errs: []svcErr{adminNotFound, {services.ErrAdminNoTwoFactor, 400, services.ErrAdminNoTwoFactor.Message}, {services.ErrAdminSelfReset, 400, services.ErrAdminSelfReset.Message}, unavailable},
-		expect: func(m svcs, err error) {
-			m.admins.ResetTwoFactor(anyCtx, actor, "a2").Return(err)
+		expect: func(svcs svcs, err error) {
+			svcs.admins.ResetTwoFactor(anyCtx, actor, "a2").Return(err)
 		},
 		check: wantSuccess,
 	},
 	"DELETE admin": {
 		method: "DELETE", path: config.AdminPath("a2", ""), errs: []svcErr{adminNotFound, adminSelf, lastAdmin, unavailable},
-		expect: func(m svcs, err error) {
-			m.admins.RemoveAdmin(anyCtx, actor, "a2").Return(err)
+		expect: func(svcs svcs, err error) {
+			svcs.admins.RemoveAdmin(anyCtx, actor, "a2").Return(err)
 		},
 		check: wantSuccess,
 	},
 }
 
 // TestCreateAdminInviteBody covers every CreateAdminInvite rule: the address
 // is normalised and the name defaults to its local part.
-func TestCreateAdminInviteBody(t *testing.T) {
-	t.Parallel()
+func TestCreateAdminInviteBody(test *testing.T) {
+	test.Parallel()
 	long := strings.Repeat("x", 244) + "@example.com" // 256 characters
 	for _, email := range []string{"", "   ", "not an email", "Ada <ada@example.com>", "ada@", "@example.com", long} {
-		wantInvalid(t, "POST", r.AdminInvites, `{"email":"`+email+`","name":"Ada"}`, map[string]string{"email": emailRule})
+		wantInvalid(test, "POST", routes.AdminInvites, `{"email":"`+email+`","name":"Ada"}`, map[string]string{"email": emailRule})
 	}
-	wantInvalid(t, "POST", r.AdminInvites, `{}`, map[string]string{"email": emailRule})
-	wantInvalid(t, "POST", r.AdminInvites, `{"email":"ada@example.com","name":"`+strings.Repeat("é", 256)+`"}`, map[string]string{"name": "must be at most 255 characters"})
-	wantInvalid(t, "POST", r.AdminInvites, `{"email":"ada","name":"`+strings.Repeat("é", 256)+`"}`, map[string]string{"email": emailRule, "name": "must be at most 255 characters"})
+	wantInvalid(test, "POST", routes.AdminInvites, `{}`, map[string]string{"email": emailRule})
+	wantInvalid(test, "POST", routes.AdminInvites, `{"email":"ada@example.com","name":"`+strings.Repeat("é", 256)+`"}`, map[string]string{"name": "must be at most 255 characters"})
+	wantInvalid(test, "POST", routes.AdminInvites, `{"email":"ada","name":"`+strings.Repeat("é", 256)+`"}`, map[string]string{"email": emailRule, "name": "must be at most 255 characters"})
 
 	valid := []struct {
 		body string
@@ -108,9 +108,9 @@ func TestCreateAdminInviteBody(t *testing.T) {
 		{`{"email":"ada@example.com","name":"` + strings.Repeat("é", 255) + `"}`, validate.CreateAdminInvite{Email: "ada@example.com", Name: strings.Repeat("é", 255)}},
 		{`{"email":"` + long[1:] + `","name":"X"}`, validate.CreateAdminInvite{Email: long[1:], Name: "X"}}, // 255 characters
 	}
-	for _, c := range valid {
-		wantOK(t, "POST", r.AdminInvites, c.body, func(m svcs) {
-			m.admins.InviteAdmin(anyCtx, actor, c.want)
+	for _, testCase := range valid {
+		wantOK(test, "POST", routes.AdminInvites, testCase.body, func(svcs svcs) {
+			svcs.admins.InviteAdmin(anyCtx, actor, testCase.want)
 		})
 	}
 }

apps/admin/internal/api/analytics_test.go

@@ -15,13 +15,13 @@ var overview = services.Overview{
 
 var analyticsRoutes = map[string]route{
 	"GET analytics": {
-		method: "GET", path: r.Analytics,
-		expect: func(m svcs, err error) {
-			m.analytics.Overview(anyCtx, actor).Return(overview, err)
+		method: "GET", path: routes.Analytics,
+		expect: func(svcs svcs, err error) {
+			svcs.analytics.Overview(anyCtx, actor).Return(overview, err)
 		},
-		check: func(t *testing.T, res apitest.Response) {
-			if got := apitest.Data[services.Overview](t, res); got != overview {
-				t.Errorf("data = %s", res.Data)
+		check: func(test *testing.T, res apitest.Response) {
+			if got := apitest.Data[services.Overview](test, res); got != overview {
+				test.Errorf("data = %s", res.Data)
 			}
 		},
 	},

apps/admin/internal/api/audit_test.go

@@ -19,30 +19,30 @@ var banEntry = services.AuditEntry{
 
 var auditRoutes = map[string]route{
 	"GET audit": {
-		method: "GET", path: r.Audit, invalidPath: r.Audit + "?offset=-1", field: "offset",
-		expect: func(m svcs, err error) {
-			m.audit.List(anyCtx, actor, validate.ListAuditLog{Limit: validate.DefaultLimit}).Return(services.AuditPage{Entries: []services.AuditEntry{banEntry}, Total: 1, Limit: 25}, err)
+		method: "GET", path: routes.Audit, invalidPath: routes.Audit + "?offset=-1", field: "offset",
+		expect: func(svcs svcs, err error) {
+			svcs.audit.List(anyCtx, actor, validate.ListAuditLog{Limit: validate.DefaultLimit}).Return(services.AuditPage{Entries: []services.AuditEntry{banEntry}, Total: 1, Limit: 25}, err)
 		},
-		check: func(t *testing.T, res apitest.Response) {
-			got := apitest.Data[services.AuditPage](t, res)
+		check: func(test *testing.T, res apitest.Response) {
+			got := apitest.Data[services.AuditPage](test, res)
 			if len(got.Entries) != 1 || got.Total != 1 || got.Limit != 25 {
-				t.Fatalf("data = %s", res.Data)
+				test.Fatalf("data = %s", res.Data)
 			}
-			e := got.Entries[0]
-			if e.ID != 7 || e.Action != "user.ban" || e.TargetLabel != banEntry.TargetLabel || e.Details["reason"] != "spam" || !e.CreatedAt.Equal(banEntry.CreatedAt) {
-				t.Errorf("entry = %+v", e)
+			entry := got.Entries[0]
+			if entry.ID != 7 || entry.Action != "user.ban" || entry.TargetLabel != banEntry.TargetLabel || entry.Details["reason"] != "spam" || !entry.CreatedAt.Equal(banEntry.CreatedAt) {
+				test.Errorf("entry = %+v", entry)
 			}
 		},
 	},
 }
 
 // TestListAuditLogQuery: filters are trimmed, the page size defaults, and the
 // bounds are inclusive.
-func TestListAuditLogQuery(t *testing.T) {
-	t.Parallel()
+func TestListAuditLogQuery(test *testing.T) {
+	test.Parallel()
 	cases := []struct {
-		q    url.Values
-		want validate.ListAuditLog
+		values url.Values
+		want   validate.ListAuditLog
 	}{
 		{url.Values{"limit": {"0"}}, validate.ListAuditLog{Limit: 25}},
 		{
@@ -54,20 +54,20 @@ func TestListAuditLogQuery(t *testing.T) {
 			validate.ListAuditLog{Action: strings.Repeat("a", 64), TargetID: strings.Repeat("t", 255), Limit: 1},
 		},
 	}
-	for _, c := range cases {
-		wantOK(t, "GET", r.Audit+"?"+c.q.Encode(), "", func(m svcs) {
-			m.audit.List(anyCtx, actor, c.want)
+	for _, testCase := range cases {
+		wantOK(test, "GET", routes.Audit+"?"+testCase.values.Encode(), "", func(svcs svcs) {
+			svcs.audit.List(anyCtx, actor, testCase.want)
 		})
 	}
 }
 
 // TestListAuditLogInvalid covers every ListAuditLog rule.
-func TestListAuditLogInvalid(t *testing.T) {
-	t.Parallel()
+func TestListAuditLogInvalid(test *testing.T) {
+	test.Parallel()
 	limit := map[string]string{"limit": "must be 1-100"}
 	cases := []struct {
-		q    string
-		want map[string]string
+		query string
+		want  map[string]string
 	}{
 		{"limit=1000", limit},
 		{"limit=101", limit},
@@ -81,7 +81,7 @@ func TestListAuditLogInvalid(t *testing.T) {
 			"action": "must be at most 64 characters", "targetId": "must be at most 255 characters", "limit": "must be 1-100", "offset": "must be 0 or more",
 		}},
 	}
-	for _, c := range cases {
-		wantInvalid(t, "GET", r.Audit+"?"+c.q, "", c.want)
+	for _, testCase := range cases {
+		wantInvalid(test, "GET", routes.Audit+"?"+testCase.query, "", testCase.want)
 	}
 }