From 00fa067c1c1eb6b3ee3812b60069aae61dad9c89 Mon Sep 17 00:00:00 2001 From: Keegan Smith Date: Fri, 14 Aug 2026 08:38:22 +0000 Subject: [PATCH] refactor/go: adopt current standard library idioms The current Go toolchain can express several loops and string operations more directly. Apply the validated automated rewrites while excluding promoted-field selectors and the unsafe index-loop transformation. Amp-Thread-ID: https://ampcode.com/threads/T-019fff44-5b25-709a-a557-5b8daee9cf75 --- api.go | 40 +++++++++---------- cmd/zoekt-local-sync/index.go | 5 ++- cmd/zoekt-mirror-gerrit/main.go | 2 +- cmd/zoekt-mirror-gitiles/cgit.go | 2 +- .../main_test.go | 2 +- cmd/zoekt-webserver/main.go | 2 +- grpc/internalerrs/common_test.go | 5 +-- index/btree.go | 4 +- index/builder.go | 5 ++- index/builder_test.go | 2 +- index/index_test.go | 2 +- index/limit.go | 5 ++- index/limit_test.go | 2 +- index/score.go | 2 +- languages/enry_vendored.go | 4 +- 15 files changed, 43 insertions(+), 41 deletions(-) diff --git a/api.go b/api.go index 6160302a2..39eba4f5e 100644 --- a/api.go +++ b/api.go @@ -482,25 +482,25 @@ func (s *Stats) Zero() bool { return true } - return !(s.ContentBytesLoaded > 0 || - s.IndexBytesLoaded > 0 || - s.Crashes > 0 || - s.FileCount > 0 || - s.FilesConsidered > 0 || - s.FilesLoaded > 0 || - s.FilesSkipped > 0 || - s.FilesSkippedDueToCancellation > 0 || - s.MatchCount > 0 || - s.NgramMatches > 0 || - s.NgramLookups > 0 || - s.ShardFilesConsidered > 0 || - s.ShardsScanned > 0 || - s.ShardsSkipped > 0 || - s.ShardsSkippedFilter > 0 || - s.Wait > 0 || - s.MatchTreeConstruction > 0 || - s.MatchTreeSearch > 0 || - s.RegexpsConsidered > 0) + return s.ContentBytesLoaded <= 0 && + s.IndexBytesLoaded <= 0 && + s.Crashes <= 0 && + s.FileCount <= 0 && + s.FilesConsidered <= 0 && + s.FilesLoaded <= 0 && + s.FilesSkipped <= 0 && + s.FilesSkippedDueToCancellation <= 0 && + s.MatchCount <= 0 && + s.NgramMatches <= 0 && + s.NgramLookups <= 0 && + s.ShardFilesConsidered <= 0 && + s.ShardsScanned <= 0 && + s.ShardsSkipped <= 0 && + s.ShardsSkippedFilter <= 0 && + s.Wait <= 0 && + s.MatchTreeConstruction <= 0 && + s.MatchTreeSearch <= 0 && + s.RegexpsConsidered <= 0 } // Progress contains information about the global progress of the running search query. @@ -930,7 +930,7 @@ type RepoListField int const ( RepoListFieldRepos RepoListField = 0 - RepoListFieldReposMap = 2 + RepoListFieldReposMap RepoListField = 2 ) type ListOptions struct { diff --git a/cmd/zoekt-local-sync/index.go b/cmd/zoekt-local-sync/index.go index bf5caf935..6ad87d703 100644 --- a/cmd/zoekt-local-sync/index.go +++ b/cmd/zoekt-local-sync/index.go @@ -21,6 +21,7 @@ import ( "io/fs" "os" "path/filepath" + "slices" "sort" "strings" "text/tabwriter" @@ -133,8 +134,8 @@ func removeShard(path string) error { var errs []error // Remove the optional metadata sidecar first. If deletion then fails, the // shard remains self-consistent and can be retried on the next sync. - for i := len(paths) - 1; i >= 0; i-- { - path := paths[i] + for _, path := range slices.Backward(paths) { + if err := os.Remove(path); err != nil && !errors.Is(err, fs.ErrNotExist) { errs = append(errs, err) } diff --git a/cmd/zoekt-mirror-gerrit/main.go b/cmd/zoekt-mirror-gerrit/main.go index 71d45c812..4a9b513d4 100644 --- a/cmd/zoekt-mirror-gerrit/main.go +++ b/cmd/zoekt-mirror-gerrit/main.go @@ -168,7 +168,7 @@ func main() { } for k, v := range *page { - if !*active || "ACTIVE" == v.State { + if !*active || v.State == "ACTIVE" { projects[k] = v } skip = skip + 1 diff --git a/cmd/zoekt-mirror-gitiles/cgit.go b/cmd/zoekt-mirror-gitiles/cgit.go index 91fcebb62..a4a9b7a71 100644 --- a/cmd/zoekt-mirror-gitiles/cgit.go +++ b/cmd/zoekt-mirror-gitiles/cgit.go @@ -46,7 +46,7 @@ func normalizedGet(u *url.URL) ([]byte, error) { return nil, err } - c = bytes.Replace(c, []byte{'\n'}, []byte{' '}, -1) + c = bytes.ReplaceAll(c, []byte{'\n'}, []byte{' '}) return c, nil } diff --git a/cmd/zoekt-sourcegraph-indexserver/main_test.go b/cmd/zoekt-sourcegraph-indexserver/main_test.go index ef3ada98e..da4c22319 100644 --- a/cmd/zoekt-sourcegraph-indexserver/main_test.go +++ b/cmd/zoekt-sourcegraph-indexserver/main_test.go @@ -305,7 +305,7 @@ func TestDefaultGRPCServiceConfigurationSyntax(t *testing.T) { if !result.Valid() { var errs strings.Builder for _, err := range result.Errors() { - errs.WriteString(fmt.Sprintf("- %s\n", err)) + fmt.Fprintf(&errs, "- %s\n", err) } t.Fatalf("default service config is invalid:\n%s", errs.String()) diff --git a/cmd/zoekt-webserver/main.go b/cmd/zoekt-webserver/main.go index bd69fd2ea..549907824 100644 --- a/cmd/zoekt-webserver/main.go +++ b/cmd/zoekt-webserver/main.go @@ -233,7 +233,7 @@ func main() { if *hostCustomization != "" { s.HostCustomQueries = map[string]string{} - for _, h := range strings.SplitN(*hostCustomization, ",", -1) { + for _, h := range strings.Split(*hostCustomization, ",") { if len(h) == 0 { continue } diff --git a/grpc/internalerrs/common_test.go b/grpc/internalerrs/common_test.go index aef587a24..ef3a65543 100644 --- a/grpc/internalerrs/common_test.go +++ b/grpc/internalerrs/common_test.go @@ -3,7 +3,6 @@ package internalerrs import ( "context" "errors" - "fmt" "sort" "strings" "testing" @@ -479,8 +478,8 @@ func TestMassageIntoStatusErr(t *testing.T) { t.Errorf("Expected ok to be %v, but got %v", tc.expectedOk, ok) } - expectedStatusString := fmt.Sprintf("%s", tc.expected) - actualStatusString := fmt.Sprintf("%s", result) + expectedStatusString := tc.expected.String() + actualStatusString := result.String() if diff := cmp.Diff(expectedStatusString, actualStatusString); diff != "" { t.Fatalf("Unexpected status string (-want +got):\n%s", diff) diff --git a/index/btree.go b/index/btree.go index e2c98f592..df94c7535 100644 --- a/index/btree.go +++ b/index/btree.go @@ -272,12 +272,12 @@ func (bt *btree) String() string { case *leaf: return case *innerNode: - s += fmt.Sprintf("[") + s += "[" for _, key := range nd.keys { s += fmt.Sprintf("%d,", key) } s = s[:len(s)-1] // remove trailing comma - s += fmt.Sprintf("]") + s += "]" } }) diff --git a/index/builder.go b/index/builder.go index cd30c9980..d221981fc 100644 --- a/index/builder.go +++ b/index/builder.go @@ -29,6 +29,7 @@ import ( "reflect" "runtime" "runtime/pprof" + "slices" "sort" "strconv" "strings" @@ -530,8 +531,8 @@ func (o *Options) FindAllShards() []string { // IgnoreSizeMax determines whether the max size should be ignored. func (o *Options) IgnoreSizeMax(name string) bool { // A pattern match will override preceding pattern matches. - for i := len(o.LargeFiles) - 1; i >= 0; i-- { - pattern := strings.TrimSpace(o.LargeFiles[i]) + for _, v := range slices.Backward(o.LargeFiles) { + pattern := strings.TrimSpace(v) negated, validatedPattern := checkIsNegatePattern(pattern) if m, _ := doublestar.PathMatch(validatedPattern, name); m { diff --git a/index/builder_test.go b/index/builder_test.go index b162da017..d8bf8ee13 100644 --- a/index/builder_test.go +++ b/index/builder_test.go @@ -1090,7 +1090,7 @@ func testFileRankAspect(t *testing.T, c filerankCase) { print := func(ds []*Document) string { var r strings.Builder for _, d := range ds { - r.WriteString(fmt.Sprintf("%v, ", d)) + fmt.Fprintf(&r, "%v, ", d) } return r.String() } diff --git a/index/index_test.go b/index/index_test.go index 33e509f07..236ff7dae 100644 --- a/index/index_test.go +++ b/index/index_test.go @@ -4093,7 +4093,7 @@ func BenchmarkScoreChunkMatches(b *testing.B) { ctx := context.Background() var builder strings.Builder for i := range 1000 { - builder.WriteString(fmt.Sprintf("line-%d one one one two two two three three three four four four five five\n", i)) + fmt.Fprintf(&builder, "line-%d one one one two two two three three three four four four five five\n", i) } searcher := searcherForTest(b, testShardBuilder(b, nil, diff --git a/index/limit.go b/index/limit.go index 9de2b1eab..31065be34 100644 --- a/index/limit.go +++ b/index/limit.go @@ -2,6 +2,7 @@ package index import ( "log" + "slices" "github.com/sourcegraph/zoekt" ) @@ -97,8 +98,8 @@ func limitChunkMatches(file *zoekt.FileMatch, limit int) int { // a trailing newline. n := cm.Ranges[len(cm.Ranges)-1].End.LineNumber - cm.Ranges[limit-1].End.LineNumber if n > 0 { - for b := len(cm.Content) - 1; b >= 0; b-- { - if cm.Content[b] == '\n' { + for b, v := range slices.Backward(cm.Content) { + if v == '\n' { n -= 1 } if n == 0 { diff --git a/index/limit_test.go b/index/limit_test.go index 0e9dff270..170eb9221 100644 --- a/index/limit_test.go +++ b/index/limit_test.go @@ -75,7 +75,7 @@ func TestLimitMatches(t *testing.T) { // 1 line of context. cm.Content = append(cm.Content, []byte("context\n")...) - for i := 0; i < numRanges; i += 1 { + for i := range numRanges { cm.Ranges = append(cm.Ranges, zoekt.Range{ // We only provide LineNumber as that's all that's // relevant. diff --git a/index/score.go b/index/score.go index 0bfdde2cf..4c1c51bae 100644 --- a/index/score.go +++ b/index/score.go @@ -394,7 +394,7 @@ func (d *indexData) scoreFileBM25(fileMatch *zoekt.FileMatch, doc uint32, cands // To make the debug output easier to read, we split the score into the query dependent score and the tiebreaker fileMatch.Debug = fmt.Sprintf("bm25-score: %.2f (low-priority: %t) <- sum-termFrequencies: %d, length-ratio: %.2f", score, lowPriority, sumTF, L) if boosted { - fileMatch.Debug += fmt.Sprintf(" (boosted)") + fileMatch.Debug += " (boosted)" } } } diff --git a/languages/enry_vendored.go b/languages/enry_vendored.go index 8bc442341..5ff922055 100644 --- a/languages/enry_vendored.go +++ b/languages/enry_vendored.go @@ -8,8 +8,8 @@ import "strings" // convertToAliasKey is vendored from go-enry to make sure // we're normalizing strings the same way. func convertToAliasKey(langName string) string { - ak := strings.SplitN(langName, `,`, 2)[0] - ak = strings.Replace(ak, ` `, `_`, -1) + ak, _, _ := strings.Cut(langName, `,`) + ak = strings.ReplaceAll(ak, ` `, `_`) ak = strings.ToLower(ak) return ak }