apply go fix modernizations across the workspace

`go fix` carries the modernize analyzers now, and the tree had drifted behind
them. This is the mechanical result, reviewed rather than trusted: the tool is
capable of rewriting code into something that no longer tests or does what it
did, so every non-test change was read individually and the concurrency-bearing
packages were re-run under -race.

Production code, four changes, all semantics-preserving:

  - leases/manager.go: wg.Add(1) + go + defer wg.Done() becomes wg.Go. The
    comment above that function turns on Add happening before the goroutine
    starts, so that a Wait cannot return before the worker has run. wg.Go does
    the Add synchronously on the calling goroutine, so the invariant it
    describes still holds.
  - auth/token/handler.go: strings.Fields -> strings.FieldsSeq, same splitting,
    iterated rather than allocated.
  - hold/gc/gc.go: a hand-written map copy -> maps.Copy.
  - hold/pds/scan_broadcaster.go: three-clause loop -> range over int.

The rest are tests. The one worth naming is carstore_contention_test.go, where a
careless rewrite could have quietly stopped exercising contention: go fix
converted the reader and side-table goroutines to loopWG.Go but correctly
declined to touch the writer loop, which passes its index as a parameter. The
writer/reader/side-table shape and the stop channel are unchanged, so the test
still contends over the same carstore transactions.

Verified: go build for hold and appview, `make lint` 0 issues, the deploy and
credential-helper modules 0 issues, `make test` green across all 43 packages,
and -race green on leases, hold/pds, hold/gc and auth/token. The scanner module's
two lint findings are unchanged from HEAD and are in files go fix never touched.

Kept separate from the HTTP/2 commit so that one stays readable, and so this can
be reverted on its own if a modernization turns out to matter.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TA9D4DjaLZTvzQ7dJbu4eg
This commit is contained in:
Evan Jarrett
2026-09-08 22:38:01 -05:00
co-authored by Claude Opus 5
parent 416ba4a2eb
commit 8d7ccd7cb7
18 changed files with 41 additions and 48 deletions
+1 -1
View File
@@ -1522,7 +1522,7 @@ func writeRemoteCloudInit(ip, script string) error {
// for cloud-init to complete before returning.
func waitForSetup(ip, name string) error {
fmt.Printf(" %s (%s): waiting for SSH...\n", name, ip)
for i := 0; i < 30; i++ {
for i := range 30 {
_, err := runSSH(ip, "echo ssh_ready", false)
if err == nil {
break
+8 -8
View File
@@ -76,13 +76,13 @@ func cmdStatus(token string) error {
if err != nil {
fmt.Printf(" Service: unreachable\n")
} else {
lines := strings.Split(strings.TrimSpace(output), "\n")
for _, line := range lines {
lines := strings.SplitSeq(strings.TrimSpace(output), "\n")
for line := range lines {
line = strings.TrimSpace(line)
if line == "active" || line == "inactive" {
fmt.Printf(" Service: %s\n", line)
} else if strings.HasPrefix(line, "health:") {
fmt.Printf(" Health: %s\n", strings.TrimPrefix(line, "health:"))
} else if after, ok := strings.CutPrefix(line, "health:"); ok {
fmt.Printf(" Health: %s\n", after)
}
}
}
@@ -101,13 +101,13 @@ func cmdStatus(token string) error {
if err != nil {
fmt.Printf(" Service: unreachable\n")
} else {
lines := strings.Split(strings.TrimSpace(output), "\n")
for _, line := range lines {
lines := strings.SplitSeq(strings.TrimSpace(output), "\n")
for line := range lines {
line = strings.TrimSpace(line)
if line == "active" || line == "inactive" {
fmt.Printf(" Service: %s\n", line)
} else if strings.HasPrefix(line, "health:") {
fmt.Printf(" Health: %s\n", strings.TrimPrefix(line, "health:"))
} else if after, ok := strings.CutPrefix(line, "health:"); ok {
fmt.Printf(" Health: %s\n", after)
}
}
}
+1 -1
View File
@@ -31,7 +31,7 @@ func seedIndexedDeviceRows(t *testing.T, store *DeviceStore, did, handle string,
t.Fatalf("GenerateFromPassword: %v", err)
}
for i := 0; i < n; i++ {
for i := range n {
hash := append([]byte(nil), base...)
// Vary the tail so secret_hash stays UNIQUE, keeping the base64 alphabet.
hash[len(hash)-1] = byte('a' + (i % 26))
+1 -1
View File
@@ -723,7 +723,7 @@ func TestDeviceStore_LegacyDeviceBackfills(t *testing.T) {
func TestDeviceStore_ValidateDoesNotScanIndexedRows(t *testing.T) {
store := setupTestDB(t)
createTestUser(t, store, "did:plc:alice123", "alice.bsky.social")
for i := 0; i < 5; i++ {
for i := range 5 {
newDeviceForTest(t, store, "did:plc:alice123", "alice.bsky.social", fmt.Sprintf("Device %d", i))
}
+2 -2
View File
@@ -282,7 +282,7 @@ func TestSubmitShedsWhenQueueIsFull(t *testing.T) {
// One job occupies the worker, one fills the single buffer slot, the
// third has nowhere to go.
accepted := 0
for i := 0; i < 3; i++ {
for i := range 3 {
if q.Submit(testRequest(fmt.Sprintf("at://did:plc:testuser/io.atcr.manifest/%d", i))) {
accepted++
}
@@ -315,7 +315,7 @@ func TestWaitDrainsAndDoesNotLeak(t *testing.T) {
return nil
})
for i := 0; i < 4; i++ {
for i := range 4 {
if !q.Submit(testRequest(fmt.Sprintf("at://did:plc:testuser/io.atcr.manifest/drain%d", i))) {
t.Fatalf("Submit %d rejected", i)
}
@@ -209,7 +209,7 @@ func TestCredHelpersKeyIsRegistryNotSite(t *testing.T) {
func offendingLines(s, needle string) string {
var out []string
for _, line := range strings.Split(s, "\n") {
for line := range strings.SplitSeq(s, "\n") {
if strings.Contains(line, needle) {
out = append(out, " "+line)
}
+2 -4
View File
@@ -126,11 +126,9 @@ var errNotAcquired = errors.New("leases: not acquired")
// could then return before the worker has even started, let alone released its
// lease.
func (m *Manager) Go(ctx context.Context, name string, fn func(context.Context) error) {
m.wg.Add(1)
go func() {
defer m.wg.Done()
m.wg.Go(func() {
m.Run(ctx, name, fn)
}()
})
}
// Wait blocks until every worker started with Go has stopped and released its
+1 -1
View File
@@ -391,7 +391,7 @@ func collectScopes(values []string) []string {
var scopes []string
seen := make(map[string]bool)
for _, value := range values {
for _, scope := range strings.Fields(value) {
for scope := range strings.FieldsSeq(value) {
if seen[scope] {
continue
}
+3 -3
View File
@@ -258,7 +258,7 @@ func TestMigrateV2toV3_DIDCollisionIsDeterministic(t *testing.T) {
}`
// Repeat: a single pass can pass by luck when the bug is map-order dependent.
for i := 0; i < 50; i++ {
for i := range 50 {
dir := setupConfigDir(t)
writeConfigFile(t, dir, v2)
@@ -297,7 +297,7 @@ func TestMigrateV2toV3_DIDCollisionIsDeterministic(t *testing.T) {
func TestMigrateV2toV3_CollisionDonatesSecret(t *testing.T) {
// Looped: with map-order-dependent code this is a coin flip, so a single
// pass can pass by luck.
for i := 0; i < 50; i++ {
for i := range 50 {
testCollisionDonatesSecret(t, i)
}
}
@@ -394,7 +394,7 @@ func TestFindIsDeterministic(t *testing.T) {
if want == nil {
t.Fatal("find returned nil for a known handle")
}
for i := 0; i < 50; i++ {
for i := range 50 {
if got := newReg().find("shared.com"); got.DID != want.DID {
t.Fatalf("iteration %d: find returned %s, want %s consistently", i, got.DID, want.DID)
}
+1 -1
View File
@@ -22,7 +22,7 @@ func assertPoolSettings(t *testing.T, label string, db *sql.DB, wantJournal stri
_ = c.Close()
}
}()
for i := 0; i < n; i++ {
for i := range n {
c, err := db.Conn(ctx)
if err != nil {
t.Fatalf("%s: open conn %d: %v", label, i, err)
+4 -4
View File
@@ -31,8 +31,8 @@ func makeBlocks(t *testing.T, n int, salt string) (cid.Cid, map[cid.Cid]blockfor
t.Helper()
blks := make(map[cid.Cid]blockformat.Block, n)
var root cid.Cid
for i := 0; i < n; i++ {
b := blockformat.NewBlock([]byte(fmt.Sprintf("%s-block-%d-%s", salt, i, strings.Repeat("x", 2048))))
for i := range n {
b := blockformat.NewBlock(fmt.Appendf(nil, "%s-block-%d-%s", salt, i, strings.Repeat("x", 2048)))
blks[b.Cid()] = b
if i == 0 {
root = b.Cid()
@@ -50,11 +50,11 @@ func writeShards(t *testing.T, sqs *SQLiteStore, writers, iters int) error {
var mu sync.Mutex
var firstErr error
for w := 0; w < writers; w++ {
for w := range writers {
wg.Add(1)
go func(w int) {
defer wg.Done()
for i := 0; i < iters; i++ {
for i := range iters {
salt := fmt.Sprintf("w%d-i%d", w, i)
root, blks := makeBlocks(t, 12, salt)
rev := fmt.Sprintf("rev-%03d-%03d", w, i)
+2 -3
View File
@@ -9,6 +9,7 @@ import (
"fmt"
"io"
"log/slog"
"maps"
"net"
"net/http"
"net/url"
@@ -136,9 +137,7 @@ func (c *auxOrphanClock) snapshot() map[string]time.Time {
defer c.mu.Unlock()
out := make(map[string]time.Time, len(c.since))
for k, v := range c.since {
out[k] = v
}
maps.Copy(out, c.since)
return out
}
+7 -11
View File
@@ -75,11 +75,11 @@ func TestHoldPDSConcurrentRepoWritesAndSideTable(t *testing.T) {
stop := make(chan struct{})
// Repo record writers.
for w := 0; w < writers; w++ {
for w := range writers {
writeWG.Add(1)
go func(w int) {
defer writeWG.Done()
for i := 0; i < iters; i++ {
for i := range iters {
rkey := fmt.Sprintf("w%d-layer-%03d", w, i)
rec := atproto.NewLayerRecord(
fmt.Sprintf("sha256:%064x", w*1000+i),
@@ -97,10 +97,8 @@ func TestHoldPDSConcurrentRepoWritesAndSideTable(t *testing.T) {
}
// Repo readers, hitting the carstore's read transactions concurrently.
for r := 0; r < 2; r++ {
loopWG.Add(1)
go func() {
defer loopWG.Done()
for range 2 {
loopWG.Go(func() {
for {
select {
case <-stop:
@@ -112,13 +110,11 @@ func TestHoldPDSConcurrentRepoWritesAndSideTable(t *testing.T) {
return
}
}
}()
})
}
// The side subsystem, writing its own table on its own goroutine.
loopWG.Add(1)
go func() {
defer loopWG.Done()
loopWG.Go(func() {
for i := 0; ; i++ {
select {
case <-stop:
@@ -133,7 +129,7 @@ func TestHoldPDSConcurrentRepoWritesAndSideTable(t *testing.T) {
return
}
}
}()
})
writersDone := make(chan struct{})
go func() {
+2 -2
View File
@@ -872,7 +872,7 @@ func TestSendBackfillMsg_AbortsOnDisconnect(t *testing.T) {
// The subscriber uses a 1-slot buffer with no reader so the backfill is
// reliably blocked in the send when Unsubscribe fires.
func TestBackfillFromDatabase_ConcurrentUnsubscribe(t *testing.T) {
for i := 0; i < 25; i++ {
for range 25 {
func() {
dbPath := t.TempDir() + "/events.db"
broadcaster := NewEventBroadcaster("did:web:hold.example.com", 500, dbPath)
@@ -880,7 +880,7 @@ func TestBackfillFromDatabase_ConcurrentUnsubscribe(t *testing.T) {
ctx := context.Background()
testCID, _ := cid.Decode("bafyreib2rxk3rkhh5ylyxj3x3gathxt3s32qvwj2lf3qg4kmzr6b7teqke")
for j := 0; j < 50; j++ {
for range 50 {
broadcaster.Broadcast(ctx, &RepoEvent{
NewRoot: testCID,
Rev: "rev",
+1 -1
View File
@@ -905,7 +905,7 @@ func (sb *ScanBroadcaster) selectSubscriberLocked() *ScanSubscriber {
best := -1
bestScore := 0.0
for i := 0; i < n; i++ {
for i := range n {
idx := (sb.nextIdx + i) % n
sub := sb.subscribers[idx]
capacity := sub.effectiveCapacity()
@@ -761,7 +761,7 @@ func TestScanDrain_StopsAtTheSubscriberCapacity(t *testing.T) {
sub.capacity = 2
var seqs []int64
for i := 0; i < 5; i++ {
for range 5 {
seqs = append(seqs, seedJob(t, sb, "sha256:backlog", originProactive))
}
+1 -1
View File
@@ -520,7 +520,7 @@ func TestScanDispatchQueue_ReturnsUndeliverableJobsToPending(t *testing.T) {
sub.capacity = 4 // the send buffer, not capacity, is the constraint here
var seqs []int64
for i := 0; i < 4; i++ {
for range 4 {
seqs = append(seqs, seedJobWithDigest(t, sb, "sha256:burst"))
}
+2 -2
View File
@@ -114,7 +114,7 @@ func newTestScanSubscriber(t *testing.T, sb *ScanBroadcaster, bufSize int) *Scan
func seedPendingJobs(t *testing.T, sb *ScanBroadcaster, n int) {
t.Helper()
for i := 0; i < n; i++ {
for i := range n {
_, err := sb.db.Exec(`
INSERT INTO scan_jobs
(manifest_digest, repository, tag, user_did, user_handle,
@@ -226,7 +226,7 @@ func TestScanUnsubscribe_MarksItsOwnJobsOnce(t *testing.T) {
// The subscriber uses a 1-slot buffer with no reader so the drain is reliably
// blocked in the send when Unsubscribe fires.
func TestScanDrainPendingJobs_ConcurrentUnsubscribe(t *testing.T) {
for i := 0; i < 25; i++ {
for range 25 {
func() {
sb := newTestScanBroadcaster(t)
sub := newTestScanSubscriber(t, sb, 1)